-
Notifications
You must be signed in to change notification settings - Fork 223
Add Static Location and Live Location Support #3531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't test a lot, since a had constant crashes (shared details on Slack). Let's figure that out first, then I will do another round.
...treamChat/Components/CustomAttachments/LocationAttachment/LocationDetailViewController.swift
Outdated
Show resolved
Hide resolved
5e5a9ab
to
70218c2
Compare
Generated by 🚫 Danger |
SDK Size
|
Sources/StreamChat/Controllers/ChannelController/ChannelController.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChat/Controllers/ChannelController/ChannelController.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChat/Controllers/MessageController/MessageController.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChat/Controllers/ChannelController/ChannelController.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChat/Controllers/ChannelController/ChannelController.swift
Show resolved
Hide resolved
Sources/StreamChat/Controllers/ChannelController/ChannelController.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChat/Controllers/ChannelController/ChannelController.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChat/Controllers/CurrentUserController/CurrentUserController.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Left few comments, let me know what you think.
...eamChat/Components/CustomAttachments/LocationAttachment/LocationAttachmentSnapshotView.swift
Outdated
Show resolved
Hide resolved
...eamChat/Components/CustomAttachments/LocationAttachment/LocationAttachmentSnapshotView.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChat/Controllers/MessageController/MessageController.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChat/Controllers/CurrentUserController/CurrentUserController.swift
Show resolved
Hide resolved
...eamChat/Components/CustomAttachments/LocationAttachment/LocationAttachmentSnapshotView.swift
Show resolved
Hide resolved
...eamChat/Components/CustomAttachments/LocationAttachment/LocationAttachmentSnapshotView.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! ✅
...eamChat/Components/CustomAttachments/LocationAttachment/LocationAttachmentSnapshotView.swift
Show resolved
Hide resolved
502d6b5
to
f1a4fac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
TestTools/StreamChatTestTools/Mocks/StreamChat/Controllers/ChatChannelController_Mock.swift (1)
58-74
: Capture thelocation
argument to aid assertions in tests
createNewMessage
already records invocation count (createNewMessageCallCount
) but does not persist the arguments passed in.
The newly-addedlocation
parameter follows the same pattern and is effectively discarded, limiting the mock’s usefulness for tests that need to verify the value propagated to the SDK.Add a stored property (or extend an existing tuple) to retain the last
location
value – mirroring what is already done forupdateDraftMessage_text
.+ // Stores the last location passed to `createNewMessage` + var createNewMessage_location: NewLocationInfo? ... override func createNewMessage( messageId: MessageId? = nil, text: String, pinning: MessagePinning? = nil, @@ restrictedVisibility: [UserId] = [], location: NewLocationInfo? = nil, extraData: [String : RawJSON] = [:], completion: ((Result<MessageId, Error>) -> Void)? = nil ) { createNewMessageCallCount += 1 + createNewMessage_location = location }This tiny addition keeps mocks transparent and makes writing expectations around static/live-location flows straightforward.
No behavioural change, zero production impact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Sources/StreamChat/Controllers/ChannelController/ChannelController.swift
(11 hunks)Sources/StreamChat/Controllers/CurrentUserController/CurrentUserController.swift
(9 hunks)Sources/StreamChat/Models/Location/NewLocationInfo.swift
(1 hunks)Sources/StreamChat/Workers/ChannelUpdater.swift
(2 hunks)StreamChat.xcodeproj/project.pbxproj
(41 hunks)TestTools/StreamChatTestTools/Mocks/StreamChat/Controllers/ChatChannelController_Mock.swift
(1 hunks)TestTools/StreamChatTestTools/SpyPattern/Spy/DatabaseContainer_Spy.swift
(4 hunks)Tests/StreamChatTests/Controllers/MessageController/MessageController_Tests.swift
(1 hunks)Tests/StreamChatTests/Workers/MessageUpdater_Tests.swift
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Sources/StreamChat/Models/Location/NewLocationInfo.swift
🚧 Files skipped from review as they are similar to previous changes (7)
- Sources/StreamChat/Workers/ChannelUpdater.swift
- StreamChat.xcodeproj/project.pbxproj
- Sources/StreamChat/Controllers/CurrentUserController/CurrentUserController.swift
- Tests/StreamChatTests/Controllers/MessageController/MessageController_Tests.swift
- Sources/StreamChat/Controllers/ChannelController/ChannelController.swift
- Tests/StreamChatTests/Workers/MessageUpdater_Tests.swift
- TestTools/StreamChatTestTools/SpyPattern/Spy/DatabaseContainer_Spy.swift
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test LLC (Debug)
- GitHub Check: Build LLC + UI (Xcode 15)
- GitHub Check: Build Test App and Frameworks
- GitHub Check: Metrics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Tests/StreamChatTests/Workers/CurrentUserUpdater_Tests.swift (1)
804-812
: Endpoint assertion looks correct – consider also asserting query parameters.The test verifies only the path and HTTP method. If the endpoint ever gains mandatory query parameters (e.g. pagination, device-id filter), the test will stay green while the call is broken.
Appending an assertion onendpoint?.queryItems
now will guard against such regressions with negligible cost.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
TestTools/StreamChatTestTools/TestData/DummyData/SharedLocationPayload.swift
(1 hunks)Tests/StreamChatTests/Workers/CurrentUserUpdater_Tests.swift
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- TestTools/StreamChatTestTools/TestData/DummyData/SharedLocationPayload.swift
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Automated Code Review
- GitHub Check: Test LLC (Debug)
- GitHub Check: Build Test App and Frameworks
- GitHub Check: Build LLC + UI (Xcode 15)
- GitHub Check: Metrics
…m-chat-swift into add/location-attachments
…m-chat-swift into add/location-attachments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
DemoApp/StreamChat/Components/CustomAttachments/DemoComposerVC.swift
(1 hunks)DemoApp/StreamChat/Components/CustomAttachments/LocationAttachment/LocationDetailViewController.swift
(2 hunks)Sources/StreamChat/APIClient/Endpoints/Payloads/ChannelListPayload.swift
(5 hunks)Sources/StreamChat/Database/DTOs/ChannelConfigDTO.swift
(3 hunks)Sources/StreamChat/Database/StreamChatModel.xcdatamodeld/StreamChatModel.xcdatamodel/contents
(4 hunks)TestTools/StreamChatTestTools/Fixtures/JSONs/Channel.json
(1 hunks)TestTools/StreamChatTestTools/Fixtures/JSONs/ChannelPayload.json
(1 hunks)TestTools/StreamChatTestTools/Mocks/Models + Extensions/ChatChannel_Mock.swift
(2 hunks)TestTools/StreamChatTestTools/TestData/DummyData/ChannelPayload.swift
(1 hunks)Tests/StreamChatTests/APIClient/Endpoints/Payloads/ChannelListPayload_Tests.swift
(1 hunks)Tests/StreamChatTests/Database/DTOs/ChannelDTO_Tests.swift
(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- TestTools/StreamChatTestTools/Fixtures/JSONs/ChannelPayload.json
- Tests/StreamChatTests/APIClient/Endpoints/Payloads/ChannelListPayload_Tests.swift
- Tests/StreamChatTests/Database/DTOs/ChannelDTO_Tests.swift
- TestTools/StreamChatTestTools/Fixtures/JSONs/Channel.json
- Sources/StreamChat/APIClient/Endpoints/Payloads/ChannelListPayload.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- DemoApp/StreamChat/Components/CustomAttachments/DemoComposerVC.swift
- Sources/StreamChat/Database/StreamChatModel.xcdatamodeld/StreamChatModel.xcdatamodel/contents
- DemoApp/StreamChat/Components/CustomAttachments/LocationAttachment/LocationDetailViewController.swift
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test LLC (Debug)
- GitHub Check: Build LLC + UI (Xcode 15)
- GitHub Check: Build Test App and Frameworks
- GitHub Check: Automated Code Review
- GitHub Check: Metrics
🔇 Additional comments (3)
TestTools/StreamChatTestTools/Mocks/Models + Extensions/ChatChannel_Mock.swift (1)
22-23
: LGTM! Mock configuration properly updated for location sharing support.The new
sharedLocationsEnabled
parameter is correctly added with appropriate defaults and properly passed to the initializer, maintaining consistency with the existing mock pattern.Also applies to: 42-43
Sources/StreamChat/Database/DTOs/ChannelConfigDTO.swift (2)
28-28
: Property declaration follows Core Data conventions.The
sharedLocationsEnabled
property is correctly declared as@NSManaged
following the same pattern as other Boolean properties in the DTO.
46-46
: Conversion methods properly updated.The
sharedLocationsEnabled
property is correctly included in bothasModel()
andasDTO()
conversion methods, ensuring proper bidirectional data flow between the domain model and Core Data representation.Also applies to: 88-88
TestTools/StreamChatTestTools/TestData/DummyData/ChannelPayload.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
Sources/StreamChat/APIClient/Endpoints/Payloads/ChannelListPayload.swift
(8 hunks)Sources/StreamChat/APIClient/Endpoints/Payloads/IdentifiableModel.swift
(1 hunks)Sources/StreamChat/Database/DTOs/ChannelDTO.swift
(4 hunks)Sources/StreamChat/Database/DTOs/MessageDTO.swift
(11 hunks)Sources/StreamChat/Database/DTOs/SharedLocationDTO.swift
(1 hunks)Sources/StreamChat/Database/DatabaseSession.swift
(4 hunks)Sources/StreamChat/Database/StreamChatModel.xcdatamodeld/StreamChatModel.xcdatamodel/contents
(5 hunks)Sources/StreamChat/Models/Channel.swift
(7 hunks)StreamChat.xcodeproj/project.pbxproj
(41 hunks)TestTools/StreamChatTestTools/Fixtures/JSONs/Channel.json
(2 hunks)TestTools/StreamChatTestTools/Mocks/Models + Extensions/ChatChannel_Mock.swift
(8 hunks)TestTools/StreamChatTestTools/Mocks/StreamChat/Database/DatabaseSession_Mock.swift
(4 hunks)TestTools/StreamChatTestTools/TestData/DummyData/ChannelPayload.swift
(2 hunks)TestTools/StreamChatTestTools/TestData/DummyData/XCTestCase+Dummy.swift
(2 hunks)Tests/StreamChatTests/APIClient/Endpoints/Payloads/ChannelListPayload_Tests.swift
(3 hunks)Tests/StreamChatTests/APIClient/Endpoints/Payloads/IdentifiablePayload_Tests.swift
(1 hunks)Tests/StreamChatTests/Database/DTOs/ChannelDTO_Tests.swift
(9 hunks)Tests/StreamChatTests/WebSocketClient/EventMiddlewares/ChannelReadUpdaterMiddleware_Tests.swift
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- Tests/StreamChatTests/WebSocketClient/EventMiddlewares/ChannelReadUpdaterMiddleware_Tests.swift
- TestTools/StreamChatTestTools/TestData/DummyData/XCTestCase+Dummy.swift
🚧 Files skipped from review as they are similar to previous changes (12)
- Tests/StreamChatTests/APIClient/Endpoints/Payloads/ChannelListPayload_Tests.swift
- Sources/StreamChat/APIClient/Endpoints/Payloads/IdentifiableModel.swift
- Sources/StreamChat/Models/Channel.swift
- StreamChat.xcodeproj/project.pbxproj
- TestTools/StreamChatTestTools/Mocks/Models + Extensions/ChatChannel_Mock.swift
- Sources/StreamChat/APIClient/Endpoints/Payloads/ChannelListPayload.swift
- TestTools/StreamChatTestTools/Mocks/StreamChat/Database/DatabaseSession_Mock.swift
- Tests/StreamChatTests/Database/DTOs/ChannelDTO_Tests.swift
- TestTools/StreamChatTestTools/TestData/DummyData/ChannelPayload.swift
- Sources/StreamChat/Database/DatabaseSession.swift
- Sources/StreamChat/Database/DTOs/MessageDTO.swift
- Sources/StreamChat/Database/StreamChatModel.xcdatamodeld/StreamChatModel.xcdatamodel/contents
🧰 Additional context used
🪛 Biome (1.9.4)
TestTools/StreamChatTestTools/Fixtures/JSONs/Channel.json
[error] 1595-1597: expected ,
but instead found "active_live_locations"
Remove "active_live_locations"
(parse)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test LLC (Debug)
- GitHub Check: Build Test App and Frameworks
- GitHub Check: Build LLC + UI (Xcode 15)
- GitHub Check: Metrics
🔇 Additional comments (10)
Tests/StreamChatTests/APIClient/Endpoints/Payloads/IdentifiablePayload_Tests.swift (1)
407-408
: LGTM! Test data updated to reflect new payload structure.The addition of
activeLiveLocations: []
to theChannelPayload
initialization correctly updates the test data to align with the new location sharing features. This ensures test compatibility with the enhanced payload structure.TestTools/StreamChatTestTools/Fixtures/JSONs/Channel.json (1)
1323-1323
: LGTM! Well-structured location sharing test data.The addition of
shared_locations
config flag andactive_live_locations
array provides comprehensive test data for the new location sharing features. The location data structure includes all necessary fields that align with the DTO implementation.Also applies to: 1595-1607
Sources/StreamChat/Database/DTOs/ChannelDTO.swift (3)
77-77
: LGTM! Correct Core Data relationship declaration.The
activeLiveLocations
property follows the established pattern for Core Data to-many relationships in this codebase.
362-364
: LGTM! Correct persistence implementation.The active live locations are properly saved from the payload using the established pattern of mapping each payload item to a DTO and collecting them into a Set.
603-603
: LGTM! Proper model conversion and initialization.The active live locations are correctly converted from DTOs to model objects and passed to the ChatChannel initializer, maintaining consistency between the persistence and model layers.
Also applies to: 637-638
Sources/StreamChat/Database/DTOs/SharedLocationDTO.swift (5)
8-16
: LGTM! Well-structured Core Data entity.The SharedLocationDTO class properly declares all necessary properties with appropriate types for location data persistence. The relationship to MessageDTO is correctly established.
18-30
: LGTM! Proper change propagation implementation.The
willSave()
override correctly ensures that location data changes are propagated to the associated message, triggering necessary UI updates. The guards and dirty-marking technique follow established patterns in this codebase.
32-62
: LGTM! Consistent static method implementation.The static methods for loading and creating SharedLocationDTO instances follow the established patterns used throughout the codebase, including proper caching support and fetch request configuration.
65-78
: LGTM! Correct model conversion implementation.The
asModel()
method properly validates the DTO state and converts all properties to the SharedLocation model, following the established conversion patterns in the codebase.
80-97
: LGTM! Proper context extension for persistence.The
saveLocation
method follows the established pattern for saving payload data to DTOs, properly handling creation/updates and mapping all properties correctly.
440ab55
to
2bd4923
Compare
|
🔗 Issue Links
Resolves https://linear.app/stream/issue/IOS-578/location-attachment
🎯 Goal
Adds support for static and live location attachments in the Low-Level Client SDK.
The UI has been implemented in the Demo App to demonstrate how to use the new location APIs.
📝 Summary
New APIs:
ChatChannelController
sendStaticLocation()
- Sends a static location message to the channel.startLiveLocationSharing()
- Starts a live location-sharing message in the channel.ChatMessageController
partialUpdateMessage()
- Updates the message partially. (It was missing from the SDK)stopLiveLocationSharing()
- Stops sharing the live location attachment if it has one.CurrentChatUserController
updateLiveLocation()
- Updates the location of all active live location messages for the current user.loadActiveLiveLocationMessages()
- Loads all active locations of the current user. Should be called only once.CurrentChatUserControllerDelegate
didStartSharingLiveLocation()
- Notifies whenever the current user is sharing any live location.didStopSharingLiveLocation()
- Notifies whenever the current user stopped/expired all live locations.didChangeActiveLiveLocationMessages()
- Notifies whenever the current user's live location messages change.didFailToUpdateLiveLocation()
- Called whenever a live location failed to update. Mostly should be used for debugging.Throttler
ChatMessage
sharedLocation
- Returns the location if it has one, either live or static.🛠 Implementation
The SDK at the moment only handles updating the location attachments. The location tracking should be provided by the App. Something like the
LocationProvider
in the Demo App should be implemented by the customer.Creating a location attachment
In order to create a new message with a location attachment, the developer can use the
ChannelController.sendStaticLocation()
or theChannelController.startSharingLiveLocation()
.Sending location updates (Live Location)
The customer is responsible for sending new location updates to the SDK. This is done through the
CurrentChatUserController.updateLiveLocation()
method. This method will update all the current user's active location attachments. Internally, it uses aactiveLiveLocationMessagesObserver
that keeps track of the active location attachments of the current user. These changes are also available to the customer through theCurrentChatUserControllerDelegate
to make it easier for the developer to know when it should track location updates and when it can turn them off.Stopping live location attachment
ChatMessageController.stopLiveLocationSharing()
: Stops a live location attachment in the given message if it has an active location attachment.Overall Data Flow
🎨 Showcase
static.mp4
live.mp4
🧪 Manual Testing Notes
Note: To simulate live location updates in the Simulator, with the Simulator selected, Go to
Features > Location > City Bicycle Ride
in the top bar of the Mac.Precondition: Select the Frankfurt C2 environemnt to test the location feature.
Send a static location message ✅
Send a live location message ✅
Receiving a live location message ✅
Active live location is ended when the last update is after expiration ✅
Active live location is ended after the expiration is reached ❌
Active live location from another user ends after the expiration is reached ❌
☑️ Contributor Checklist
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
Chores